Skip to content

[X86][GlobalIsel] Reland G_ISFPCLASS #165848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[X86][GlobalIsel] Reland G_ISFPCLASS #165848

wants to merge 2 commits into from
+309 −163

Conversation

@mahesh-attarde
Copy link
Contributor

@mahesh-attarde mahesh-attarde commented Oct 31, 2025 •
edited
Loading

Earlier Attempt to support G_ISFPCLASS #162232 broke EXPENSIVE_CHECKS and was reverted.
Current Patch addresses failure in EXPENSIVE_CHECKS.

More about Regression #162232 (comment),
After Instruction Selection Machine Code Verification failed due to invalid register class for subregister indexing.
e.g.

# After InstructionSelect
# Machine code for function isnone_f: IsSSA, TracksLiveness, Legalized, RegBankSelected, Selected
bb.1.entry:
  %5:gr32 = MOV32r0 implicit-def dead $eflags
  %4:gr8 = COPY %5.sub_8bit:gr32
  $al = COPY %4:gr8
  RET 0, implicit $al

*** Bad machine code: Invalid register class for subregister index ***
- function:    isnone_f
- basic block: %bb.1 entry (0x5e1917110858)
- instruction: %4:gr8 = COPY %5.sub_8bit:gr32
- operand 1:   %5.sub_8bit:gr32
Register class GR32 does not fully support subreg index sub_8bit
LLVM ERROR: Found 1 machine code errors.

SDAG handled %5 definition with RC gr32_abcd and use with RC sub_8bit:gr32_abcd, This was perform right in InstEmitter::emitSubReg based on EXTRACT_SUBREG node information.
In GISEL we did not have similar representation or lowering, Hence we do similar transform "After Isel" while optimizing redundant copy instructions. (May not be best place, I am open for suggestions)

@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2025 •
edited
Loading

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-x86

Author: Mahesh-Attarde (mahesh-attarde)

Changes

Earlier Attempt #162232 broke EXPENSIVE_CHECKS and was reverted.
Current Patch addresses failure in EXPENSIVE_CHECKS.

More about Regression, After Instruction Selection Machine Code Verification failed.

# After InstructionSelect
# Machine code for function isnone_f: IsSSA, TracksLiveness, Legalized, RegBankSelected, Selected
bb.1.entry:
  %5:gr32 = MOV32r0 implicit-def dead $eflags
  %4:gr8 = COPY %5.sub_8bit:gr32
  $al = COPY %4:gr8
  RET 0, implicit $al

*** Bad machine code: Invalid register class for subregister index ***
- function:    isnone_f
- basic block: %bb.1 entry (0x5e1917110858)
- instruction: %4:gr8 = COPY %5.sub_8bit:gr32
- operand 1:   %5.sub_8bit:gr32
Register class GR32 does not fully support subreg index sub_8bit
LLVM ERROR: Found 1 machine code errors.

SDAG handled %5 definition with RC sub_8bit:gr32_abcd, This was perform right in InstEmitter::emitSubReg based on EXTRACT_SUBREG node information.
In GISEL we did not have similar representation or lowering, Hence we do similar transform "After Isel" while optimizing redundant copy instructions. (May not be best place, I am open for suggestions)

Patch is 24.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/165848.diff

3 Files Affected:

  • (modified) llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp (+10)
  • (modified) llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp (+2)
  • (modified) llvm/test/CodeGen/X86/isel-fpclass.ll (+298-163)
diff --git a/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp b/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
index 2dd22c8a7e8ba4..5a499a92d75ac7 100644
--- a/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
@@ -238,6 +238,7 @@ bool InstructionSelect::selectMachineFunction(MachineFunction &MF) {
       continue;
     }
     // Try to find redundant copies b/w vregs of the same register class.
+    // Try to constrain the register class of the copy operand.
     for (auto MII = MBB.rbegin(), End = MBB.rend(); MII != End;) {
       MachineInstr &MI = *MII;
       ++MII;
@@ -254,6 +255,15 @@ bool InstructionSelect::selectMachineFunction(MachineFunction &MF) {
           MI.eraseFromParent();
         }
       }
+      auto CopyOpd = MI.getOperand(1);
+      if (CopyOpd.getSubReg() != 0) {
+        auto VReg = CopyOpd.getReg();
+        const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo();
+        const TargetRegisterClass *RC = TRI.getSubClassWithSubReg(
+            MRI.getRegClass(VReg), CopyOpd.getSubReg());
+        MRI.constrainRegClass(
+            VReg, RC); // 3rd Arg MinRCSize in DAG is 4, we used 0 here.
+      }
     }
   }
 
diff --git a/llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp b/llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp
index e792b1bce3c5c7..32b81e36a51263 100644
--- a/llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp
+++ b/llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp
@@ -590,6 +590,8 @@ X86LegalizerInfo::X86LegalizerInfo(const X86Subtarget &STI,
       .lower();
 
   // fp intrinsics
+  getActionDefinitionsBuilder(G_IS_FPCLASS).lower();
+
   getActionDefinitionsBuilder({G_INTRINSIC_ROUNDEVEN, G_INTRINSIC_TRUNC})
       .scalarize(0)
       .minScalar(0, LLT::scalar(32))
diff --git a/llvm/test/CodeGen/X86/isel-fpclass.ll b/llvm/test/CodeGen/X86/isel-fpclass.ll
index df04b673d82238..c7c54a74f5f877 100644
--- a/llvm/test/CodeGen/X86/isel-fpclass.ll
+++ b/llvm/test/CodeGen/X86/isel-fpclass.ll
@@ -1,11 +1,10 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
-; RUN: llc < %s -mtriple=i686-linux | FileCheck %s -check-prefixes=X86
+; RUN: llc < %s -mtriple=i686-linux | FileCheck %s -check-prefixes=X86,X86-SDAGISEL
 ; RUN: llc < %s -mtriple=x86_64-linux | FileCheck %s -check-prefixes=X64,X64-SDAGISEL
 ; RUN: llc < %s -mtriple=i686-linux -fast-isel -fast-isel-abort=1  | FileCheck %s -check-prefixes=X86-FASTISEL
 ; RUN: llc < %s -mtriple=x86_64-linux -fast-isel -fast-isel-abort=1  | FileCheck %s -check-prefixes=X64,X64-FASTISEL
-; RUN: llc < %s -mtriple=i686-linux -global-isel -global-isel-abort=2  | FileCheck %s -check-prefixes=X86
-; RUN: llc < %s -mtriple=x86_64-linux -global-isel -global-isel-abort=2  | FileCheck %s -check-prefixes=X64,X64-GISEL
-
+; RUN: llc < %s -mtriple=i686-linux -global-isel -global-isel-abort=1  | FileCheck %s -check-prefixes=X86,X86-GISEL
+; RUN: llc < %s -mtriple=x86_64-linux -global-isel -global-isel-abort=1  | FileCheck %s -check-prefixes=X64-GISEL
 define i1 @isnone_f(float %x) nounwind {
 ; X86-LABEL: isnone_f:
 ; X86:       # %bb.0: # %entry
@@ -51,27 +50,16 @@ entry:
 }
 
 define i1 @issignaling_f(float %x) nounwind {
-; X86-LABEL: issignaling_f:
-; X86:       # %bb.0:
-; X86-NEXT:    movl $2147483647, %eax # imm = 0x7FFFFFFF
-; X86-NEXT:    andl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
-; X86-NEXT:    setl %cl
-; X86-NEXT:    cmpl $2139095041, %eax # imm = 0x7F800001
-; X86-NEXT:    setge %al
-; X86-NEXT:    andb %cl, %al
-; X86-NEXT:    retl
-;
-; X64-LABEL: issignaling_f:
-; X64:       # %bb.0:
-; X64-NEXT:    movd %xmm0, %eax
-; X64-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
-; X64-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
-; X64-NEXT:    setl %cl
-; X64-NEXT:    cmpl $2139095041, %eax # imm = 0x7F800001
-; X64-NEXT:    setge %al
-; X64-NEXT:    andb %cl, %al
-; X64-NEXT:    retq
+; X64-SDAGISEL-LABEL: issignaling_f:
+; X64-SDAGISEL:       # %bb.0:
+; X64-SDAGISEL-NEXT:    movd %xmm0, %eax
+; X64-SDAGISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-SDAGISEL-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
+; X64-SDAGISEL-NEXT:    setl %cl
+; X64-SDAGISEL-NEXT:    cmpl $2139095041, %eax # imm = 0x7F800001
+; X64-SDAGISEL-NEXT:    setge %al
+; X64-SDAGISEL-NEXT:    andb %cl, %al
+; X64-SDAGISEL-NEXT:    retq
 ;
 ; X86-FASTISEL-LABEL: issignaling_f:
 ; X86-FASTISEL:       # %bb.0:
@@ -87,26 +75,42 @@ define i1 @issignaling_f(float %x) nounwind {
 ; X86-FASTISEL-NEXT:    andb %cl, %al
 ; X86-FASTISEL-NEXT:    popl %ecx
 ; X86-FASTISEL-NEXT:    retl
+;
+; X64-FASTISEL-LABEL: issignaling_f:
+; X64-FASTISEL:       # %bb.0:
+; X64-FASTISEL-NEXT:    movd %xmm0, %eax
+; X64-FASTISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-FASTISEL-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
+; X64-FASTISEL-NEXT:    setl %cl
+; X64-FASTISEL-NEXT:    cmpl $2139095041, %eax # imm = 0x7F800001
+; X64-FASTISEL-NEXT:    setge %al
+; X64-FASTISEL-NEXT:    andb %cl, %al
+; X64-FASTISEL-NEXT:    retq
+;
+; X64-GISEL-LABEL: issignaling_f:
+; X64-GISEL:       # %bb.0:
+; X64-GISEL-NEXT:    movd %xmm0, %eax
+; X64-GISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-GISEL-NEXT:    xorl %ecx, %ecx
+; X64-GISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-GISEL-NEXT:    seta %dl
+; X64-GISEL-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
+; X64-GISEL-NEXT:    setb %al
+; X64-GISEL-NEXT:    andb %dl, %al
+; X64-GISEL-NEXT:    orb %cl, %al
+; X64-GISEL-NEXT:    retq
    %a0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 1)  ; "snan"
    ret i1 %a0
 }
 
  define i1 @isquiet_f(float %x) nounwind {
-; X86-LABEL: isquiet_f:
-; X86:       # %bb.0: # %entry
-; X86-NEXT:    movl $2147483647, %eax # imm = 0x7FFFFFFF
-; X86-NEXT:    andl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
-; X86-NEXT:    setge %al
-; X86-NEXT:    retl
-;
-; X64-LABEL: isquiet_f:
-; X64:       # %bb.0: # %entry
-; X64-NEXT:    movd %xmm0, %eax
-; X64-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
-; X64-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
-; X64-NEXT:    setge %al
-; X64-NEXT:    retq
+; X64-SDAGISEL-LABEL: isquiet_f:
+; X64-SDAGISEL:       # %bb.0: # %entry
+; X64-SDAGISEL-NEXT:    movd %xmm0, %eax
+; X64-SDAGISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-SDAGISEL-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
+; X64-SDAGISEL-NEXT:    setge %al
+; X64-SDAGISEL-NEXT:    retq
 ;
 ; X86-FASTISEL-LABEL: isquiet_f:
 ; X86-FASTISEL:       # %bb.0: # %entry
@@ -119,27 +123,37 @@ define i1 @issignaling_f(float %x) nounwind {
 ; X86-FASTISEL-NEXT:    setge %al
 ; X86-FASTISEL-NEXT:    popl %ecx
 ; X86-FASTISEL-NEXT:    retl
+;
+; X64-FASTISEL-LABEL: isquiet_f:
+; X64-FASTISEL:       # %bb.0: # %entry
+; X64-FASTISEL-NEXT:    movd %xmm0, %eax
+; X64-FASTISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-FASTISEL-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
+; X64-FASTISEL-NEXT:    setge %al
+; X64-FASTISEL-NEXT:    retq
+;
+; X64-GISEL-LABEL: isquiet_f:
+; X64-GISEL:       # %bb.0: # %entry
+; X64-GISEL-NEXT:    movd %xmm0, %eax
+; X64-GISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-GISEL-NEXT:    xorl %ecx, %ecx
+; X64-GISEL-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
+; X64-GISEL-NEXT:    setae %al
+; X64-GISEL-NEXT:    orb %cl, %al
+; X64-GISEL-NEXT:    retq
  entry:
    %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 2)  ; "qnan"
    ret i1 %0
 }
 
 define i1 @not_isquiet_f(float %x) nounwind {
-; X86-LABEL: not_isquiet_f:
-; X86:       # %bb.0: # %entry
-; X86-NEXT:    movl $2147483647, %eax # imm = 0x7FFFFFFF
-; X86-NEXT:    andl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
-; X86-NEXT:    setl %al
-; X86-NEXT:    retl
-;
-; X64-LABEL: not_isquiet_f:
-; X64:       # %bb.0: # %entry
-; X64-NEXT:    movd %xmm0, %eax
-; X64-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
-; X64-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
-; X64-NEXT:    setl %al
-; X64-NEXT:    retq
+; X64-SDAGISEL-LABEL: not_isquiet_f:
+; X64-SDAGISEL:       # %bb.0: # %entry
+; X64-SDAGISEL-NEXT:    movd %xmm0, %eax
+; X64-SDAGISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-SDAGISEL-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
+; X64-SDAGISEL-NEXT:    setl %al
+; X64-SDAGISEL-NEXT:    retq
 ;
 ; X86-FASTISEL-LABEL: not_isquiet_f:
 ; X86-FASTISEL:       # %bb.0: # %entry
@@ -152,27 +166,46 @@ define i1 @not_isquiet_f(float %x) nounwind {
 ; X86-FASTISEL-NEXT:    setl %al
 ; X86-FASTISEL-NEXT:    popl %ecx
 ; X86-FASTISEL-NEXT:    retl
+;
+; X64-FASTISEL-LABEL: not_isquiet_f:
+; X64-FASTISEL:       # %bb.0: # %entry
+; X64-FASTISEL-NEXT:    movd %xmm0, %eax
+; X64-FASTISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-FASTISEL-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
+; X64-FASTISEL-NEXT:    setl %al
+; X64-FASTISEL-NEXT:    retq
+;
+; X64-GISEL-LABEL: not_isquiet_f:
+; X64-GISEL:       # %bb.0: # %entry
+; X64-GISEL-NEXT:    movd %xmm0, %eax
+; X64-GISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-GISEL-NEXT:    xorl %ecx, %ecx
+; X64-GISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-GISEL-NEXT:    setb %dl
+; X64-GISEL-NEXT:    orb %cl, %dl
+; X64-GISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-GISEL-NEXT:    sete %cl
+; X64-GISEL-NEXT:    orb %dl, %cl
+; X64-GISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-GISEL-NEXT:    seta %dl
+; X64-GISEL-NEXT:    cmpl $2143289344, %eax # imm = 0x7FC00000
+; X64-GISEL-NEXT:    setb %al
+; X64-GISEL-NEXT:    andb %dl, %al
+; X64-GISEL-NEXT:    orb %cl, %al
+; X64-GISEL-NEXT:    retq
 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 1021)  ; ~"qnan"
   ret i1 %0
 }
 
 define i1 @isinf_f(float %x) nounwind {
-; X86-LABEL: isinf_f:
-; X86:       # %bb.0: # %entry
-; X86-NEXT:    movl $2147483647, %eax # imm = 0x7FFFFFFF
-; X86-NEXT:    andl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; X86-NEXT:    sete %al
-; X86-NEXT:    retl
-;
-; X64-LABEL: isinf_f:
-; X64:       # %bb.0: # %entry
-; X64-NEXT:    movd %xmm0, %eax
-; X64-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
-; X64-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; X64-NEXT:    sete %al
-; X64-NEXT:    retq
+; X64-SDAGISEL-LABEL: isinf_f:
+; X64-SDAGISEL:       # %bb.0: # %entry
+; X64-SDAGISEL-NEXT:    movd %xmm0, %eax
+; X64-SDAGISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-SDAGISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-SDAGISEL-NEXT:    sete %al
+; X64-SDAGISEL-NEXT:    retq
 ;
 ; X86-FASTISEL-LABEL: isinf_f:
 ; X86-FASTISEL:       # %bb.0: # %entry
@@ -185,27 +218,37 @@ define i1 @isinf_f(float %x) nounwind {
 ; X86-FASTISEL-NEXT:    sete %al
 ; X86-FASTISEL-NEXT:    popl %ecx
 ; X86-FASTISEL-NEXT:    retl
+;
+; X64-FASTISEL-LABEL: isinf_f:
+; X64-FASTISEL:       # %bb.0: # %entry
+; X64-FASTISEL-NEXT:    movd %xmm0, %eax
+; X64-FASTISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-FASTISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-FASTISEL-NEXT:    sete %al
+; X64-FASTISEL-NEXT:    retq
+;
+; X64-GISEL-LABEL: isinf_f:
+; X64-GISEL:       # %bb.0: # %entry
+; X64-GISEL-NEXT:    movd %xmm0, %eax
+; X64-GISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-GISEL-NEXT:    xorl %ecx, %ecx
+; X64-GISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-GISEL-NEXT:    sete %al
+; X64-GISEL-NEXT:    orb %cl, %al
+; X64-GISEL-NEXT:    retq
 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 516)  ; 0x204 = "inf"
   ret i1 %0
 }
 
 define i1 @not_isinf_f(float %x) nounwind {
-; X86-LABEL: not_isinf_f:
-; X86:       # %bb.0: # %entry
-; X86-NEXT:    movl $2147483647, %eax # imm = 0x7FFFFFFF
-; X86-NEXT:    andl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; X86-NEXT:    setne %al
-; X86-NEXT:    retl
-;
-; X64-LABEL: not_isinf_f:
-; X64:       # %bb.0: # %entry
-; X64-NEXT:    movd %xmm0, %eax
-; X64-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
-; X64-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; X64-NEXT:    setne %al
-; X64-NEXT:    retq
+; X64-SDAGISEL-LABEL: not_isinf_f:
+; X64-SDAGISEL:       # %bb.0: # %entry
+; X64-SDAGISEL-NEXT:    movd %xmm0, %eax
+; X64-SDAGISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-SDAGISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-SDAGISEL-NEXT:    setne %al
+; X64-SDAGISEL-NEXT:    retq
 ;
 ; X86-FASTISEL-LABEL: not_isinf_f:
 ; X86-FASTISEL:       # %bb.0: # %entry
@@ -218,24 +261,39 @@ define i1 @not_isinf_f(float %x) nounwind {
 ; X86-FASTISEL-NEXT:    setne %al
 ; X86-FASTISEL-NEXT:    popl %ecx
 ; X86-FASTISEL-NEXT:    retl
+;
+; X64-FASTISEL-LABEL: not_isinf_f:
+; X64-FASTISEL:       # %bb.0: # %entry
+; X64-FASTISEL-NEXT:    movd %xmm0, %eax
+; X64-FASTISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-FASTISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-FASTISEL-NEXT:    setne %al
+; X64-FASTISEL-NEXT:    retq
+;
+; X64-GISEL-LABEL: not_isinf_f:
+; X64-GISEL:       # %bb.0: # %entry
+; X64-GISEL-NEXT:    movd %xmm0, %eax
+; X64-GISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-GISEL-NEXT:    xorl %ecx, %ecx
+; X64-GISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-GISEL-NEXT:    setb %dl
+; X64-GISEL-NEXT:    orb %cl, %dl
+; X64-GISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-GISEL-NEXT:    seta %al
+; X64-GISEL-NEXT:    orb %dl, %al
+; X64-GISEL-NEXT:    retq
 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 507)  ; ~0x204 = "~inf"
   ret i1 %0
 }
 
 define i1 @is_plus_inf_f(float %x) nounwind {
-; X86-LABEL: is_plus_inf_f:
-; X86:       # %bb.0: # %entry
-; X86-NEXT:    cmpl $2139095040, {{[0-9]+}}(%esp) # imm = 0x7F800000
-; X86-NEXT:    sete %al
-; X86-NEXT:    retl
-;
-; X64-LABEL: is_plus_inf_f:
-; X64:       # %bb.0: # %entry
-; X64-NEXT:    movd %xmm0, %eax
-; X64-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; X64-NEXT:    sete %al
-; X64-NEXT:    retq
+; X64-SDAGISEL-LABEL: is_plus_inf_f:
+; X64-SDAGISEL:       # %bb.0: # %entry
+; X64-SDAGISEL-NEXT:    movd %xmm0, %eax
+; X64-SDAGISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-SDAGISEL-NEXT:    sete %al
+; X64-SDAGISEL-NEXT:    retq
 ;
 ; X86-FASTISEL-LABEL: is_plus_inf_f:
 ; X86-FASTISEL:       # %bb.0: # %entry
@@ -246,24 +304,34 @@ define i1 @is_plus_inf_f(float %x) nounwind {
 ; X86-FASTISEL-NEXT:    sete %al
 ; X86-FASTISEL-NEXT:    popl %ecx
 ; X86-FASTISEL-NEXT:    retl
+;
+; X64-FASTISEL-LABEL: is_plus_inf_f:
+; X64-FASTISEL:       # %bb.0: # %entry
+; X64-FASTISEL-NEXT:    movd %xmm0, %eax
+; X64-FASTISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-FASTISEL-NEXT:    sete %al
+; X64-FASTISEL-NEXT:    retq
+;
+; X64-GISEL-LABEL: is_plus_inf_f:
+; X64-GISEL:       # %bb.0: # %entry
+; X64-GISEL-NEXT:    xorl %ecx, %ecx
+; X64-GISEL-NEXT:    movd %xmm0, %eax
+; X64-GISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-GISEL-NEXT:    sete %al
+; X64-GISEL-NEXT:    orb %cl, %al
+; X64-GISEL-NEXT:    retq
 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 512)  ; 0x200 = "+inf"
   ret i1 %0
 }
 
 define i1 @is_minus_inf_f(float %x) nounwind {
-; X86-LABEL: is_minus_inf_f:
-; X86:       # %bb.0: # %entry
-; X86-NEXT:    cmpl $-8388608, {{[0-9]+}}(%esp) # imm = 0xFF800000
-; X86-NEXT:    sete %al
-; X86-NEXT:    retl
-;
-; X64-LABEL: is_minus_inf_f:
-; X64:       # %bb.0: # %entry
-; X64-NEXT:    movd %xmm0, %eax
-; X64-NEXT:    cmpl $-8388608, %eax # imm = 0xFF800000
-; X64-NEXT:    sete %al
-; X64-NEXT:    retq
+; X64-SDAGISEL-LABEL: is_minus_inf_f:
+; X64-SDAGISEL:       # %bb.0: # %entry
+; X64-SDAGISEL-NEXT:    movd %xmm0, %eax
+; X64-SDAGISEL-NEXT:    cmpl $-8388608, %eax # imm = 0xFF800000
+; X64-SDAGISEL-NEXT:    sete %al
+; X64-SDAGISEL-NEXT:    retq
 ;
 ; X86-FASTISEL-LABEL: is_minus_inf_f:
 ; X86-FASTISEL:       # %bb.0: # %entry
@@ -274,24 +342,34 @@ define i1 @is_minus_inf_f(float %x) nounwind {
 ; X86-FASTISEL-NEXT:    sete %al
 ; X86-FASTISEL-NEXT:    popl %ecx
 ; X86-FASTISEL-NEXT:    retl
+;
+; X64-FASTISEL-LABEL: is_minus_inf_f:
+; X64-FASTISEL:       # %bb.0: # %entry
+; X64-FASTISEL-NEXT:    movd %xmm0, %eax
+; X64-FASTISEL-NEXT:    cmpl $-8388608, %eax # imm = 0xFF800000
+; X64-FASTISEL-NEXT:    sete %al
+; X64-FASTISEL-NEXT:    retq
+;
+; X64-GISEL-LABEL: is_minus_inf_f:
+; X64-GISEL:       # %bb.0: # %entry
+; X64-GISEL-NEXT:    xorl %ecx, %ecx
+; X64-GISEL-NEXT:    movd %xmm0, %eax
+; X64-GISEL-NEXT:    cmpl $-8388608, %eax # imm = 0xFF800000
+; X64-GISEL-NEXT:    sete %al
+; X64-GISEL-NEXT:    orb %cl, %al
+; X64-GISEL-NEXT:    retq
 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 4)  ; "-inf"
   ret i1 %0
 }
 
 define i1 @not_is_minus_inf_f(float %x) nounwind {
-; X86-LABEL: not_is_minus_inf_f:
-; X86:       # %bb.0: # %entry
-; X86-NEXT:    cmpl $-8388608, {{[0-9]+}}(%esp) # imm = 0xFF800000
-; X86-NEXT:    setne %al
-; X86-NEXT:    retl
-;
-; X64-LABEL: not_is_minus_inf_f:
-; X64:       # %bb.0: # %entry
-; X64-NEXT:    movd %xmm0, %eax
-; X64-NEXT:    cmpl $-8388608, %eax # imm = 0xFF800000
-; X64-NEXT:    setne %al
-; X64-NEXT:    retq
+; X64-SDAGISEL-LABEL: not_is_minus_inf_f:
+; X64-SDAGISEL:       # %bb.0: # %entry
+; X64-SDAGISEL-NEXT:    movd %xmm0, %eax
+; X64-SDAGISEL-NEXT:    cmpl $-8388608, %eax # imm = 0xFF800000
+; X64-SDAGISEL-NEXT:    setne %al
+; X64-SDAGISEL-NEXT:    retq
 ;
 ; X86-FASTISEL-LABEL: not_is_minus_inf_f:
 ; X86-FASTISEL:       # %bb.0: # %entry
@@ -302,27 +380,43 @@ define i1 @not_is_minus_inf_f(float %x) nounwind {
 ; X86-FASTISEL-NEXT:    setne %al
 ; X86-FASTISEL-NEXT:    popl %ecx
 ; X86-FASTISEL-NEXT:    retl
+;
+; X64-FASTISEL-LABEL: not_is_minus_inf_f:
+; X64-FASTISEL:       # %bb.0: # %entry
+; X64-FASTISEL-NEXT:    movd %xmm0, %eax
+; X64-FASTISEL-NEXT:    cmpl $-8388608, %eax # imm = 0xFF800000
+; X64-FASTISEL-NEXT:    setne %al
+; X64-FASTISEL-NEXT:    retq
+;
+; X64-GISEL-LABEL: not_is_minus_inf_f:
+; X64-GISEL:       # %bb.0: # %entry
+; X64-GISEL-NEXT:    movd %xmm0, %eax
+; X64-GISEL-NEXT:    movl %eax, %ecx
+; X64-GISEL-NEXT:    andl $2147483647, %ecx # imm = 0x7FFFFFFF
+; X64-GISEL-NEXT:    xorl %edx, %edx
+; X64-GISEL-NEXT:    cmpl $2139095040, %ecx # imm = 0x7F800000
+; X64-GISEL-NEXT:    setb %sil
+; X64-GISEL-NEXT:    orb %dl, %sil
+; X64-GISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-GISEL-NEXT:    sete %dl
+; X64-GISEL-NEXT:    cmpl $2139095040, %ecx # imm = 0x7F800000
+; X64-GISEL-NEXT:    seta %al
+; X64-GISEL-NEXT:    orb %dl, %al
+; X64-GISEL-NEXT:    orb %sil, %al
+; X64-GISEL-NEXT:    retq
 entry:
   %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 1019)  ; ~"-inf"
   ret i1 %0
 }
 
 define i1 @isfinite_f(float %x) nounwind {
-; X86-LABEL: isfinite_f:
-; X86:       # %bb.0: # %entry
-; X86-NEXT:    movl $2147483647, %eax # imm = 0x7FFFFFFF
-; X86-NEXT:    andl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; X86-NEXT:    setl %al
-; X86-NEXT:    retl
-;
-; X64-LABEL: isfinite_f:
-; X64:       # %bb.0: # %entry
-; X64-NEXT:    movd %xmm0, %eax
-; X64-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
-; X64-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
-; X64-NEXT:    setl %al
-; X64-NEXT:    retq
+; X64-SDAGISEL-LABEL: isfinite_f:
+; X64-SDAGISEL:       # %bb.0: # %entry
+; X64-SDAGISEL-NEXT:    movd %xmm0, %eax
+; X64-SDAGISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-SDAGISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-SDAGISEL-NEXT:    setl %al
+; X64-SDAGISEL-NEXT:    retq
 ;
 ; X86-FASTISEL-LABEL: isfinite_f:
 ; X86-FASTISEL:       # %bb.0: # %entry
@@ -335,27 +429,37 @@ define i1 @isfinite_f(float %x) nounwind {
 ; X86-FASTISEL-NEXT:    setl %al
 ; X86-FASTISEL-NEXT:    popl %ecx
 ; X86-FASTISEL-NEXT:    retl
+;
+; X64-FASTISEL-LABEL: isfinite_f:
+; X64-FASTISEL:       # %bb.0: # %entry
+; X64-FASTISEL-NEXT:    movd %xmm0, %eax
+; X64-FASTISEL-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
+; X64-FASTISEL-NEXT:    cmpl $2139095040, %eax # imm = 0x7F800000
+; X64-FA...
[truncated]

@RKSimon RKSimon requested review from RKSimon, arsenm and e-kud October 31, 2025 11:23
arsenm
arsenm requested changes Oct 31, 2025
View reviewed changes
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not be touching the instruction select pass. This is purely an x86 selector bug

@mahesh-attarde
Copy link
Contributor Author

mahesh-attarde commented Nov 3, 2025 •
edited
Loading

You should not be touching the instruction select pass. This is purely an x86 selector bug

I agree, This is terrible way to approach it and needs approach you mentioned if no general use. Patch intends address same problem that was originally handled in generic InstrEmitter code.

For Selection DAG, we have done this in InstrEmitter::EmitSubregNode.
https://github.com/mahesh-attarde/llvm-project/blob/jf_fplass_spr/llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp#L517 which has generic InstrEmitter. This was unexpected lengthy debug since we dont expect opts in emitter.

My intent is discussion trade-off for following.

  1. If there is general use, should we define GISEL opcode for "G_EXTRACT_SUBREG" ?
    It seems overengineering just for X86 (or RISCV AFAIK), It seems like too trivial case to miss. So it is left out intentionally?
    Does it need broad discussion?
  2. Add post-isel fixup pass/hook to address similar adjustment ?
    Apart from Iteration, I dont see any issue if fixup lands just after instruction-select.
    This seems like narrow and easy approach.

What do you think?
@e-kud @arsenm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arsenm arsenm arsenm requested changes

@e-kud e-kud Awaiting requested review from e-kud

@RKSimon RKSimon Awaiting requested review from RKSimon

Assignees

No one assigned

Labels

backend:X86 llvm:globalisel

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mahesh-attarde @llvmbot @arsenm